Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/controller/reconciler.go`:
- Around line 705-721: Update the settings assembly around
normalizedMySQLSettings and the operator-owned invariant map to remove both
skip-log-bin and disable-log-bin aliases, including underscore forms such as
skip_log_bin, before rendering the final configuration. Ensure group and site
overrides cannot reintroduce these keys, while preserving the enforced log-bin
setting, and add coverage for both underscore and hyphen inputs.
- Around line 1305-1314: Update reconcileSiteService to assign
svc.Spec.LoadBalancerIP from site.LBIP when the effective service type is
LoadBalancer, and clear it for other service types or when the configuration
changes away from LoadBalancer. Add tests covering the reader LoadBalancer path
and clearing the field after a service-type transition.

In `@internal/controller/runner.go`:
- Around line 893-900: Move the source-convergence evaluation in the runner’s
replication-status handling ahead of the nil Replication guard, specifically for
read-only follower states. Ensure probe failures with Replication=nil and
Pending/ProbeFailed still set the Degraded condition when SourceConvergenceState
is not sourceConvergenceConverged, while preserving the existing nil-status skip
behavior for unrelated cases.

In `@internal/controller/source_convergence.go`:
- Around line 145-158: Update the rollback restart calls in the post-stop probe
and source-change failure paths within the surrounding convergence method to use
a fresh bounded context derived from the parent context, rather than the
potentially expired ctx. Create or obtain that rollback context before stopping
replication and pass it to follower.mysql.StartReplica, preserving the existing
divergence check and error returns.

In `@internal/controller/topology.go`:
- Line 1296: Bound each MySQL fencing and safety-probe phase, including the
SetSuperReadOnly call and the regions around the referenced probe blocks, with a
short child context deadline derived from the manager Poll context. Use the
existing normal-polling timeout pattern, pass the bounded context to every probe
in each phase, and ensure the child context is released after the phase
completes so stalled drivers cannot block fencing, failover, or status updates
indefinitely.

In `@internal/controller/updater.go`:
- Around line 229-240: Update the failover flow around failover.Execute so the
authority/status change and onPromoted callback occur immediately after a
successful failover, before CheckReadOnly is called. Preserve the existing error
return for CheckReadOnly failures while ensuring the promoted handoff target is
recorded even when that validation fails.
- Around line 212-220: Update the follower selection loop in the updater handoff
logic to track which followers were successfully processed by the earlier
update/probe phase, and only assign handoff to a non-drifted follower or one
whose update completed successfully; do not reselect a drifted follower solely
because a second requireDirectReplica call succeeds. Add a regression test
covering a candidate whose first probe fails and second probe succeeds,
verifying it is not handed off while stale.

In `@internal/metrics/metrics.go`:
- Around line 97-100: Add stable failover-group identity to
ReplicationSourceState by including namespace/group labels alongside site and
state, then update every metric emitter, test, and documentation reference to
supply and describe those labels consistently. Preserve the existing state-set
semantics while ensuring sites with the same name in different groups produce
distinct series.

In `@internal/playground/logs/matcher.go`:
- Around line 39-71: Update textFieldMatches to match complete
whitespace-delimited key/value tokens, requiring a valid key boundary and
ensuring the value ends before whitespace or the end of the line. Preserve
support for quoted values while rejecting key prefixes and value prefixes such
as oldsource=... and source=auto-clone-old; keep Structured’s JSON matching
unchanged.

In `@internal/state/matrix.go`:
- Around line 73-75: Update the matrix action-building flow around the
writable-site handling and PromotionCandidates population: when any writable
site with a non-promotable role is added to FenceSites, return a
fencing-specific degraded action immediately instead of allowing promotion
candidates to be populated. Preserve promotion only after a subsequent poll
confirms the site is no longer writable or has been successfully fenced.

In `@playground/chaos-scenarios.md`:
- Around line 270-277: Update Scenario 11’s adjacent Verify text to state that
recovery results in one writable site and two read-only follower sites, matching
the three deployments scaled by the injection.
- Line 1073: Update the continuous observer description to avoid claiming
immediate failure: state that s40ContinuousObservation.fail records invariant
violations during scale-down, empty-datadir detection, clone, or catch-up, with
errors reported later by stopAndCheck. Do not imply the active operation is
aborted unless the runner is changed to cancel on failure.

In `@test/envtest/reconciler_test.go`:
- Around line 369-377: Update the owner-reference assertion in the per-site
ConfigMap verification loop to validate that each ConfigMap is controlled by the
expected object using metav1.IsControlledBy(&cm, fg), rather than checking only
OwnerReferences[0].Name. Preserve the existing failure reporting and ConfigMap
lookup behavior.
- Around line 161-173: Strengthen the negative cases in the test loop around
tt.wantOK and k8sClient.Create by asserting that the error is a Kubernetes
schema-validation Invalid error, ideally also verifying the expected field path.
Keep the valid-object assertion unchanged, and ensure unrelated failures such as
namespace setup errors cannot satisfy the invalid-object branch.

---

Outside diff comments:
In `@internal/state/matrix.go`:
- Around line 71-102: Update the observation loop around coreCount so every
configured primary-candidate site increments the core-site count before state
classification, including sites with StateUnknown. Keep fencing and bucket
classification behavior unchanged, and ensure an all-unknown primary-candidate
set reaches the existing non-healthy/NoPrimary handling instead of the coreCount
== 0 Healthy path.

In `@test/envtest/reconciler_test.go`:
- Around line 485-515: Update the existing-deployment verification in the
reconciliation test to cover both dc1 and dc2. Capture each Deployment’s MySQL
container image by container name rather than assuming Containers[0], then
assert after reconciliation that both named-container images remain unchanged.

---

Nitpick comments:
In `@internal/controller/reclone_test.go`:
- Around line 93-99: Fold TestValidateRecloneRequest_ReadOnlyRecipient_OK into
the adjacent table-driven cold-reclone validation test, adding a case that
configures the recipient with SiteRoleReadOnly and uses the correct confirmation
token. Remove the standalone test and reuse the existing table setup and
assertion flow.

In `@internal/metrics/metrics_test.go`:
- Around line 39-54: Replace the one-off
TestRegisterIncludesReplicationSourceState with a table-driven registration test
covering replication source state and structured for additional metric cases.
Keep each case responsible for setting up any required labels, gathering the
registry, and asserting the expected metric family, while preserving cleanup
through ReplicationSourceState.DeleteLabelValues.

In `@test/envtest/reconciler_test.go`:
- Around line 402-413: The Service checks in the reconciler test should validate
that each administrative `-internal` Service remains cluster-internal, not just
that it exists and has an owner reference. For those services, assert the
expected internal `corev1.ServiceType` and verify external exposure fields such
as node ports and load balancer ingress are absent; leave the existing checks
for other services unchanged.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cmd/playground-chaos/reset.go`:
- Line 117: Add regression tests for the reset sequence around
normalizeFreshBaseline, verifying it runs after the first healthy baseline,
preserves scale-down → status-clear → restart → wait ordering, and stops
immediately on the first error. Cover the API-server mutation interactions using
the existing test patterns and run make test-envtest.